-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply 2to3 next
fix.
#2134
Apply 2to3 next
fix.
#2134
Conversation
Test results for commit 360c5cd merged into master
Not available for testing: python2.6 |
@@ -940,6 +940,8 @@ def next (self): # File-like object. | |||
raise StopIteration | |||
return result | |||
|
|||
next = __next__ # File-like object. (Python 2). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively we could treat this as:
def __next(self)__:
# ...
if not py3compat.PY3:
next = __next__
This would make it more obvious what needs to be ripped out if Python 2 support was ever dropped...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd side with doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I've made the changes and pushed a new commit.
Manually set `next = __next__` for Python 2 support.
Test results for commit 30e4a14 merged into master
Not available for testing: python2.6 |
This looks OK to me. If no-one objects in about a day, we can merge it. |
raise IOError('Read not supported on a write only stream.') | ||
|
||
if not py3compat.PY3: | ||
next = __next__ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good reason that this method is defined in a conditional, and not just next = __next__
? I know the method is not required in py3, but it seems cleaner to just define it.
Ah, now I just saw the conversation above. I don't really agree with the decision, but I don't feel strongly, so I'll go with you guys. Also, Python 2.6 testing is the most important here before merge. |
Test results for commit 30e4a14 merged into master
Not available for testing: python3.1 |
2.6 is happy, +1 to merge. |
Merged - thanks. |
Apply 2to3 `next` fix.
Continuing on the Python 2/3 compatibility (which I'll refer to as '2and3') from #2095 and #2100, I propose to include the changes from running
2to3 -f next
:One small issue is the behavior of
next(x)
on user defined classes:Consider
test_next.py
And run it in Python 2.7 and 3.2:
To work around this difference, I've just set
next = __next__
manually after applying the 2to3 fix. This also maintains compatibility with any third party code which callsx.next()
.